Skip to content

Conversation

@JJFlorian
Copy link

@JJFlorian JJFlorian commented Dec 5, 2025

Closes #10427

In array-api-strict>2.4.1, arithmetic operations no longer accept NumPy arrays. By inverting the assert_equal variables, we use numpy's __eq__ instead of the array-api-strict __eq__

@welcome
Copy link

welcome bot commented Dec 5, 2025

Thank you for opening this pull request! It may take us a few days to respond here, so thank you for being patient.
If you have questions, some answers may be found in our contributing guidelines.

@github-actions github-actions bot added CI Continuous Integration tools dependencies Pull requests that update a dependency file labels Dec 5, 2025
@JJFlorian
Copy link
Author

JJFlorian commented Dec 5, 2025

I don't understand why the mypy tests fail after these changes, so can't really come up with a way to fix this (except for giving an ignore to mypy :) )

@dcherian dcherian requested a review from Illviljan December 5, 2025 16:59
Copy link
Contributor

@Illviljan Illviljan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Array api strict is now typed since 2.4.0: data-apis/array-api-strict#135

This is the erroring code:

try:
import array_api_strict
except ImportError:
class DummyArrayAPINamespace:
bool = None # type: ignore[unused-ignore,var-annotated]
int32 = None # type: ignore[unused-ignore,var-annotated]
float64 = None # type: ignore[unused-ignore,var-annotated]
array_api_strict = DummyArrayAPINamespace

Redefinitions with different types is in general not a good idea for type safety. mypy choose the first one and goes from there. Since array api strict wasn't typed <2.4.0, the type was Any which made the except case valid too, not anymore though.

We usually just ignore in these optional import cases, examples are here:

try:
from dask.array import Array as DaskArray
except ImportError:
DaskArray = np.ndarray # type: ignore[misc, assignment, unused-ignore]

actual = xp_arr + 7
assert isinstance(actual.data, Array)
assert_equal(actual, expected)
assert_equal(expected, actual)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think array api strict had the right idea here.
It's not a good idea to assume different types of arrays will work with each other.

Should we instead explicitly convert to numpy?
actual = np.asarray(xp_arr + 7)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for you insight @Illviljan. Changing the array-api-strict to np arrays sounds like a good plan.

I think we want to keep the assert isinstance(xp_arr.data, Array) check after the array operations, and since actual should be a xr.DataArray, I changed it now to:

actual_np = actual.copy(data=np.asarray(actual.data))
assert_equal(actual_np, expected)

@Illviljan Illviljan added the plan to merge Final call for comments label Dec 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI Continuous Integration tools dependencies Pull requests that update a dependency file plan to merge Final call for comments

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test failures with array-api-strict 2.4.0

2 participants